-
-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A vitest example #437
base: develop
Are you sure you want to change the base?
A vitest example #437
Conversation
I believe the fail() pattern came from jasmine down to jest in the first instance, though I am not sure. In any event it leads to some strange testing behavior in which you begin to re-invent the framework from the inside out. For this reason it seems the vite team has decided not to make a pass at including it, since it is technically breaking containment even in the context of jest testing. I have done my best to take the time it needs to ascertain which of these fails() should be asserts, and which should be using the standard expectations api.
I have maintained the file-sequential execution style (which I will next attempt to take away.) tests currently run through in ~99 seconds or so. This is much longer than it needs to be, and so this is where the vitest optimizations can really have an opportunity to shine.
The test speed is up roughly 10x but my expectation is that it should be able to come lower than that. The beauty of the way test containment is implemented here means that there is not much need for isolation - which is good for simulating our use case because we make use of a lot of singletons (though thankfully not TOO much shared state) It was all relatively trivial with the exception of histories. I am intensely anxious with the implementation that has been used for document history, as it obscures control flow and does not look like it will scale - especially if we were ever to want horizontal deployment. All in all the tests are faster, and can tighten up development feedback loops BUT the un-strict nature of our existing test runners means that there are some inconsistencies in execution that I haven't yet uncovered
I still need a little guidance on what the area history behavior is supposed to be, since it is not clear to me what approach is being used - it's only two tests failing but it is indicative of my misunderstanding. All tests resolve in about 8 seconds, which is quite nice
the vite config is also expressed in TS, so it should not be exempt from linting
testing is quiiiiite noisy in testing
Thanks for adding the pagination tests!
I've merged all new needed stuff into this branch in case anyone wants to take a peak at a more up-to-date proposal Vitest runs in ~8s It should be said that this is not necessarily all down to vitest being that much faster than jest (though in my experience it is a bit faster). The real benefit comes from isolating the test states properly using vitest's fixture api. This lets us run the tests in parallel using their little rust runners under the hood. I will also note that two of the history tests fail because I do not understand the history behavior well enough to redesign the tests |
All but one test passing, Just thought I would share the outcome so my issue doesn't seem so arbitrary:
Headlines
It's a roughly 10x improvement which is frankly not what I had hoped to achieve, the tests are roughly as fast as I'd like them to be when running in a
--watch
contextObjectives
tests should run faster, and
--watch
should be a little more usable to tighten up development feedback loops. Ideally, I'd also like the test runners to have much less noise, since we use a LOT of code to scaffold test state that is not actually a part of what particular state is under examination in the test. This slows comprehension and makes the test files very bulky and cumbersome.Remaining issues
Obviously, one of the tests is still not passing so... that's not good. It's a history test, and as soon as you have forked control flows parallel testing gets a little tricky. The reason I stopped debugging the test was because I could not properly rationalize what the intended behavior was supposed to be. I need to take a longer think about the changelog system we have, but I thought I'd leave this here to explain #420 a little bit - we do not need to pursue this PR, it is not a priority at the moment